-
Notifications
You must be signed in to change notification settings - Fork 0
Add support for configuring backpressure #14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
9dbd727 to
2b889f2
Compare
| } | ||
|
|
||
| /// No backpressure will be applied to reading requests or writing responses. | ||
| public static let none: Self = .init(backing: .none) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When is this an acceptable choice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Arguably never at least in production services but I thought it made sense to still offer it? I can get rid of it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The underlying async channel doesn’t even offer that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, we don't need to offer things that nobody should ever use.
2b889f2 to
4424950
Compare
FranzBusch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I approve this for now but I'm wondering how useful just this setting is since it only applies to the outmost AsyncChannel for the request/responses. There are a few more in play here though. Let's move forward but we might want to expose more configuration here for the various bits over time.
4424950 to
5ee4044
Compare
|
For the record:
|
No description provided.